chore(i18n): fix and update CLDR to v30.0.1 #15997
Conversation
Previously, it was assumed that all currency pattern would have fraction digits. However, in [closure-library@b9155d5][1] the `agq_CM` locale was modified to have such a pattern (namely `#,##0\u00A4`). This commit modifies the parser implementation to account for pattern without a decimal point (and thus no fraction digits). [1]: google/closure-library@b9155d5#diff-02793124214ad0470ccea6f86b90d786R711
|
Do you know why the scripts broke? Because they must have worked before. |
|
I don't know why or when the scripts/tests broke. It is possible that someone run the node scripts directly from inside the There is a README.md inside |
| @@ -75,7 +75,6 @@ describe('findLocaleId', function() { | |||
| it('should not find localeId if data is missing', function() { | |||
| expect(findLocaleId('', 'num')).toBeUndefined(); | |||
| expect(findLocaleId('aa', 'datetime')).toBeUndefined(); | |||
| expect(findLocaleId('aa', 'randomType')).toBeUndefined(); | |||
There was a problem hiding this comment.
Why is this assertion bad? Or, what caused it to be bad?
There was a problem hiding this comment.
This throws an "unknown type in findLocaleId: randomType" error. This is what the next test verifies.
I assume that at some point someone decided to change the implementation, so that unknown types cause an error (vs returning undefined), but didn't update this test.
Maybe we should run these tests on CI.
i18n/src/parser.js
Outdated
| @@ -33,7 +46,12 @@ function parsePattern(pattern) { | |||
| positive = patternParts[0], | |||
| negative = patternParts[1]; | |||
|
|
|||
| var positiveParts = positive.split(DECIMAL_SEP), | |||
| // The parsing logic below assumes that there will always be a DECIMAL_SEP in the pattern. | |||
There was a problem hiding this comment.
I'm a bit confused by this. Does it mean if a locale doesn't have a DECIMAL_SEP, we add it after the last ZERO, which means it will be discarded later (because nothing comes after it)? So we only add it for the sake of parsing?
There was a problem hiding this comment.
Pretty much. We add it for the sake of parsing, because what comes after the DECIMAL_SEP determines the min/max fraction size (depending on the number of ZERO and DIGIT occurrences) and also the posSuf. For example, if we have #,##0$, we convert it to #,##0.$, which (when parsed) results in minFrac: 0, maxFrac: 0, posSuf: $ - i.e. have 0 fraction digits and tput $ after the number (posSuf).
There was a problem hiding this comment.
Ok, I see. Can you please a short summary of this to the comment? I think this will be helpful.
There was a problem hiding this comment.
Nobody reads comments. When you want to understand the code, you look at the tests 😛
(I'll update the comment.)
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Fix, i18n update.
What is the current behavior? (You can also link to an open issue here)
Locale files based on CLDR 29 and broken
i18n/scripts.What is the new behavior (if this is a feature change)?
Locale files based on CLDR 30.0.1 and working
i18n/scripts.Does this PR introduce a breaking change?
No.
Please check if the PR fulfills these requirements
Docs have been added / updated (for bug fixes / features)Other information:
Fixes #15976.